Added square wave functionality to play tone. #116

ryanskeith · 2022-05-03T17:04:58Z

Added code to enable square tone. This turns out to be much louder.
In this addition, the variable names that referred to sine waves were
generalized to _wave and _wave sample. This is a proposed solution
for issue #49. And the square wave is much louder.

Added code to enable square tone. This turns out to be much louder. In this addition, the variable names that referred to sine waves were generalized to _wave and _wave sample.

FoamyGuy · 2022-05-03T17:11:03Z

The actions check failed due to code formatting. There is a guide page here that has more information and shows how to get set up locally to run the check and have it do the formatting changes for you. https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code

We'll also need to double check whether there is enough room on the CPX builds to include this change. iirc it's a rather tight fit and this library is frozen in so increases in code size here can possibly make it not fit within that build.

ryanskeith · 2022-05-03T19:18:29Z

I feel this is completed and ready for review.

kattni · 2022-05-03T19:57:23Z

Unfortunately, I don't have a CPX with me to test this with. I can test the build sizes though.

tekktrik · 2022-05-03T20:06:47Z

Linked the relevant issue since it seems to address it.

FoamyGuy · 2022-05-03T20:11:19Z

I successfully tested the functionality with a custom build of CPX with this branch as the frozen in lib. It is a good margin louder.

I tried one of the builds that I knew from a prior PR was most limited in space and unfortunately this does push it over the limit:

make BOARD=circuitplayground_express_displayio TRANSLATION=ru
...
253564 bytes used, -124 bytes free in flash firmware space out of 253440 bytes (247.5kB).
12192 bytes used, 20576 bytes free in ram for stack and heap out of 32768 bytes (32.0kB).

Too little flash!!!

kattni · 2022-05-03T20:28:35Z

@FoamyGuy I completely forgot about the displayio build. I tested the regular build for CPX and it's fine.

@dhalbert Is there any way to shrink the displayio build? Or is that not feasible here?

ryanskeith · 2022-05-03T20:45:11Z

So close, yet so far away... This issue may need to be dropped as I am not seeing a way to reduce how to simplify the solution further. Alternatively, you could adopt to use the square wave versus sine as it is louder.

FoamyGuy · 2022-05-03T20:46:43Z

That thought of changing to use only the square wave function and remove sine crossed my mind as a possibility too. I don't necessarily have a super strong preference either way but the louder one is easier to tell when it's working depending on the frequencies used.

kattni · 2022-05-03T20:48:00Z

Let's also give Dan a chance to let us know if he can do something with the displayio build. If not, then we'll make a decision on how to move forward here.

dhalbert · 2022-05-03T20:54:26Z

Setting CIRCUITPY_ONEWIREIO = 0 in the build would recover 500-600 bytes.

dhalbert · 2022-05-03T20:58:13Z

So if you go ahead and submit this, then when we update the frozen modules the next time, I will also shrink that build.

kattni · 2022-05-03T20:59:43Z

Excellent, thank you @dhalbert!

@FoamyGuy Go ahead and review it with that in mind, thanks!

FoamyGuy · 2022-05-03T21:00:00Z

I tried the same build with CIRCUITPY_ONEWIREIO = 0

But it still came out just a tad over:

253476 bytes used, -36 bytes free in flash firmware space out of 253440 bytes (247.5kB).
12192 bytes used, 20576 bytes free in ram for stack and heap out of 32768 bytes (32.0kB).

This is with the branch from this PR and CIRCUITPY_ONEWIREIO = 0 on CPX_Displayio with ru translation.

kattni · 2022-05-03T21:01:10Z

Oof, ok. @FoamyGuy let's still review this with the assumption that we can make it fit and then make a decision. Thanks!

dhalbert · 2022-05-03T21:03:15Z

Ugh, well, I think I can find something or other.

FoamyGuy · 2022-05-03T21:05:23Z

fwiw I did also try with deleting bluefruit.py inside the frozen library and it does fit with a good amount of head-room like that:

252304 bytes used, 1136 bytes free in flash firmware space out of 253440 bytes (247.5kB).
12192 bytes used, 20576 bytes free in ram for stack and heap out of 32768 bytes (32.0kB).

(same build for CPX_Displayio ru and this branch of the library).

If we can come up with a mechanism that allows excluding files from there it could give us enough space I think.

dhalbert · 2022-05-03T21:07:34Z

adafruit_circuitplayground/circuit_playground_base.py

+        if waveform == "square":
+            self._wave = array.array("H", self._square_sample(length))
+        else:
+            self._wave = array.array("H", self._sine_sample(length))


This may shrink things enough to fit

Suggested change

if waveform == "square":

self._wave = array.array("H", self._square_sample(length))

else:

self._wave = array.array("H", self._sine_sample(length))

self._wave = array.array("H", self._square_sample(length) if waveform == "square" else self._sine_sample(length))

There may be similar refactorings in the Python code that would save a few bytes here and there.

I wasn't sure if ternary operator worked in Circuit Python. I wrote it this way if anyone else wanted to add different shaped waves down the line. If it works, great but to me the sound difference isn't that big. Square definitely sounds more ragged than sine but that could be that the code is driving it rather hard.

FoamyGuy · 2022-05-03T21:11:56Z

adafruit_circuitplayground/circuit_playground_base.py

+        for _ in range(half_length):
+            yield 0
+
+    def _generate_sample(self, length=100, waveform="sine"):


waveform could take constant values instead of a string.

I don't know if strictly better one way or the other but I tend to try to use constant number values with descriptively named variables instead of strings when I write code like this.

I'm not sure my tendencies are worth holding this up over though so we can see if anyone else has thoughts on it.

i.e usage would look something like:

circuitplayground.start_tone(234, circuitplayground.SINE_WAVEFORM)

and the value can then be a constant int rather than a string.

That's a great idea as well. Is there an enum type in circuit python?

I don't think we have enum, but we do have a const that is inherited from micropython: https://docs.circuitpython.org/en/latest/docs/library/micropython.html?#micropython.const

since the variable names may take some space as well it may be worthwhile to go with a more shorthand version like SINE_WAVE instead of SINE_WAVEFORM as well. Every bit counts to an extent here.

That is a good idea. I will look over variable names.

SINE_WAVEFORM, etc. are going to be the equivalent of strings, so you will not gain space, unfortunately, I think. The names need to be available at run time.

ryanskeith · 2022-05-03T22:07:27Z

I moved from string to number way of doing things to determine SINE or SQUARE. Let me know if it made the space cut @FoamyGuy, @kattni .

FoamyGuy · 2022-05-03T22:25:52Z

Tested that same cpx displayio+ru build on the latest version and still not quite fitting -56 bytes on the latest one.

We'll look into potential refactors to squeeze it a bit more and then eventually maybe try to have some mechanism that can exclude files that are unneeded to gain back some space.

tekktrik · 2022-05-04T16:48:27Z

Another potential fix is #117 by @Neradoc

kattni · 2022-05-09T21:14:45Z

Merging this with the understanding that we will need to tweak the build when we update the frozen modules.

@ryanskeith Thank you again!

Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 5.2.0 from 5.1.0: > Patch .pre-commit-config.yaml > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#116 from ryanskeith/add_square_wave > change discord badge > Patch: Replaced discord badge image > Update .gitignore Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 4.2.1 from 4.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#163 from dannystaple/patch-1 > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Update .gitignore > Update Black to latest. Updating https://github.com/adafruit/Adafruit_CircuitPython_IL91874 to 1.2.4 from 1.2.3: > Merge pull request adafruit/Adafruit_CircuitPython_IL91874#15 from makermelissa/master > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. > Fixed readthedocs build > Consolidate Documentation sections of README Updating https://github.com/adafruit/Adafruit_CircuitPython_GC_IOT_Core to 3.2.0 from 3.1.2: > Merge pull request adafruit/Adafruit_CircuitPython_GC_IOT_Core#23 from tannewt/use_real_ntp > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. Updating https://github.com/adafruit/Adafruit_CircuitPython_AzureIoT to 2.5.4 from 2.5.3: > Merge pull request adafruit/Adafruit_CircuitPython_AzureIoT#43 from tannewt/use_real_ntp > Merge pull request adafruit/Adafruit_CircuitPython_AzureIoT#45 from adafruit/delete-unused-license > Patch .pre-commit-config.yaml Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_iBBQ to 1.2.8 from 1.2.7: > Merge pull request adafruit/Adafruit_CircuitPython_BLE_iBBQ#9 from sabadam32/add_type_annotations_6 > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Update .gitignore > Update Black to latest. > Fixed readthedocs build > Consolidate Documentation sections of README Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32S2TFT to 1.1.0 from 1.0.1: > Update .pre-commit-config.yaml > Patch .pre-commit-config.yaml > Merge pull request adafruit/Adafruit_CircuitPython_ESP32S2TFT#6 from FoamyGuy/dont_require_secrets_if_no_network > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Merge pull request adafruit/Adafruit_CircuitPython_ESP32S2TFT#5 from FoamyGuy/fix_circup_instructions Updating https://github.com/adafruit/Adafruit_CircuitPython_OAuth2 to 1.0.7 from 1.0.6: > Merge pull request adafruit/Adafruit_CircuitPython_OAuth2#7 from ktkinsey37/typing > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. > Fixed readthedocs build > Consolidate Documentation sections of README Updating https://github.com/adafruit/Adafruit_CircuitPython_PIOASM to 0.7.3 from 0.7.2: > Patch .pre-commit-config.yaml > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#49 from jepler/undo-folder-structure > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#48 from jepler/7seg-counter Updating https://github.com/adafruit/Adafruit_CircuitPython_Simple_Text_Display to 1.2.4 from 1.2.3: > Patch .pre-commit-config.yaml > Merge pull request adafruit/Adafruit_CircuitPython_Simple_Text_Display#12 from kattni/docs-fix > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. > Fixed readthedocs build > Post-patch cleanup > Consolidate Documentation sections of README

Added square wave functionality to play tone.

2759f47

Added code to enable square tone. This turns out to be much louder. In this addition, the variable names that referred to sine waves were generalized to _wave and _wave sample.

ryanskeith added 2 commits May 3, 2022 11:31

Fixing pylint statement because of black.

1fa20bb

Removing unncessary variable.

36bbb56

dhalbert requested a review from kattni May 3, 2022 17:39

ryanskeith added 2 commits May 3, 2022 11:51

Removing duplicate code.

dd3b5a3

Blackened.

b508e65

Adding SPDX line for myself.

eb52cea

tekktrik linked an issue May 3, 2022 that may be closed by this pull request

Sine wave may be rather quiet - add option to change to square wave #49

Closed

dhalbert mentioned this pull request May 3, 2022

Update frozen modules for 7.3.0 final adafruit/circuitpython#6345

Closed

1 task

dhalbert reviewed May 3, 2022

View reviewed changes

FoamyGuy reviewed May 3, 2022

View reviewed changes

ryanskeith added 2 commits May 3, 2022 15:57

Changing from string to number format for waveform selection.

74b8dd3

Fixed linting error. Blackened.

3a4b2b3

kattni merged commit a6b5b88 into adafruit:main May 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added square wave functionality to play tone. #116

Added square wave functionality to play tone. #116

ryanskeith commented May 3, 2022

FoamyGuy commented May 3, 2022

ryanskeith commented May 3, 2022

kattni commented May 3, 2022

tekktrik commented May 3, 2022

FoamyGuy commented May 3, 2022

kattni commented May 3, 2022

ryanskeith commented May 3, 2022 •

edited

Loading

FoamyGuy commented May 3, 2022

kattni commented May 3, 2022

dhalbert commented May 3, 2022

dhalbert commented May 3, 2022

kattni commented May 3, 2022

FoamyGuy commented May 3, 2022

kattni commented May 3, 2022

dhalbert commented May 3, 2022

FoamyGuy commented May 3, 2022

dhalbert May 3, 2022

dhalbert May 3, 2022

ryanskeith May 3, 2022

FoamyGuy May 3, 2022

ryanskeith May 3, 2022

FoamyGuy May 3, 2022

FoamyGuy May 3, 2022

ryanskeith May 3, 2022

dhalbert May 3, 2022 •

edited

Loading

ryanskeith commented May 3, 2022 •

edited

Loading

FoamyGuy commented May 3, 2022

tekktrik commented May 4, 2022

kattni commented May 9, 2022

Added square wave functionality to play tone. #116

Added square wave functionality to play tone. #116

Conversation

ryanskeith commented May 3, 2022

FoamyGuy commented May 3, 2022

ryanskeith commented May 3, 2022

kattni commented May 3, 2022

tekktrik commented May 3, 2022

FoamyGuy commented May 3, 2022

kattni commented May 3, 2022

ryanskeith commented May 3, 2022 • edited Loading

FoamyGuy commented May 3, 2022

kattni commented May 3, 2022

dhalbert commented May 3, 2022

dhalbert commented May 3, 2022

kattni commented May 3, 2022

FoamyGuy commented May 3, 2022

kattni commented May 3, 2022

dhalbert commented May 3, 2022

FoamyGuy commented May 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhalbert May 3, 2022 • edited Loading

Choose a reason for hiding this comment

ryanskeith commented May 3, 2022 • edited Loading

FoamyGuy commented May 3, 2022

tekktrik commented May 4, 2022

kattni commented May 9, 2022

ryanskeith commented May 3, 2022 •

edited

Loading

dhalbert May 3, 2022 •

edited

Loading

ryanskeith commented May 3, 2022 •

edited

Loading